Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First batch of yapf auto-formatting. #630

Closed
wants to merge 1 commit into from
Closed

Conversation

nworden
Copy link
Contributor

@nworden nworden commented Apr 7, 2019

This covers all our files under app/ which don't start with a, f, m, or v.

Starts addressing issue #619.

This covers all our files under app/ which don't start with a, f, m, or v.

Starts addressing issue google#619.
@nworden
Copy link
Contributor Author

nworden commented Apr 7, 2019

I'll do tests separately. I skipped some letters because I'm doing work on admin handlers, you have a pending change for frontend_api.py, I have a pending change for model.py, and vintol mentioned he's working on views.py.

@nworden nworden requested a review from gimite April 7, 2019 17:44
@nworden
Copy link
Contributor Author

nworden commented Apr 8, 2019

Hm, I think I figured out why Jakub's PR had a lint failure: yapf had a release today, and I think it generated different output than before. I'm going to close this PR for now, and look into other auto-formatter options. yapf seemed like a good choice because it's from Google and would be sure to use the style guide we use, but maybe it'd be better to choose something more mature. Enforcing use of an auto-formatter with less-than-stable output seems like a real hassle.

@nworden nworden closed this Apr 8, 2019
@gimite
Copy link
Contributor

gimite commented Apr 8, 2019

If we cannot find a good alternative, does it make sense to stick to a specific version of yapf (and perform project-wide batch update when we feel the version is too old)?

@nworden
Copy link
Contributor Author

nworden commented Apr 9, 2019

Replied on issue #619

nworden added a commit to nworden/personfinder that referenced this pull request Apr 19, 2019
I dropped them from travis_tests earlier because it turns out it's not
really suitable for continuous testing (discussed on PR google#630), but I
forgot to take it out of tools/all_tests.
nworden added a commit that referenced this pull request Apr 19, 2019
I dropped them from travis_tests earlier because it turns out it's not
really suitable for continuous testing (discussed on PR #630), but I
forgot to take it out of tools/all_tests.
@nworden nworden deleted the yapf1 branch April 27, 2019 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants